-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[SPIRV][HLSL] Add DXC compatibility option for extension #151554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The default behaviour in DXC is to allow all extesions the compiler knows about. We did the same in clang: all extensions that clang knows about. However, this causes the shader to use different extensions because the two compilers have different sets of extensions. To avoid using a new extension when moving from DXC to Clang, we add the special DXC suboptions to `-fspv-extension". If `-fspv-extension=DXC` is used, then the available extensions will be those available in DXC.
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-clang-driver Author: Steven Perron (s-perron) ChangesThe default behaviour in DXC is to allow all extesions the compiler To avoid using a new extension when moving from DXC to Clang, we add the Full diff: https://github.com/llvm/llvm-project/pull/151554.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index eb53821bbf4d5..9471e9edcfd76 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -9428,8 +9428,12 @@ def fspv_target_env_EQ : Joined<["-"], "fspv-target-env=">, Group<dxc_Group>,
def fspv_extension_EQ
: Joined<["-"], "fspv-extension=">,
Group<dxc_Group>,
- HelpText<"Specify the available SPIR-V extensions. If this option is not "
- "specified, then all extensions are available.">;
+ HelpText<
+ "Specify the available SPIR-V extensions. If this option is not "
+ "specified, then all extensions are available. If KHR is specified, "
+ "then all KHR extensions will be available. If DXC is specified, "
+ "then all extensions implemented by the DirectXShader compiler will "
+ "be available. This option is useful for moving from DXC to Clang.">;
def fvk_use_dx_layout
: DXCFlag<"fvk-use-dx-layout">,
HelpText<"Use DirectX memory layout for Vulkan resources.">;
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 38f4643abad98..48761c254dfd8 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -173,24 +173,78 @@ bool isLegalValidatorVersion(StringRef ValVersionStr, const Driver &D) {
return true;
}
+std::string getSpirvExtOperand(llvm::StringRef SpvExtensionArg) {
+ // The extensions that are commented out are supported in DXC, but the SPIR-V
+ // backend does not know about them yet.
+ static const std::vector<std::string> DxcSupportedExtensions = {
+ "SPV_KHR_16bit_storage",
+ "SPV_KHR_device_group",
+ "SPV_KHR_fragment_shading_rate",
+ "SPV_KHR_multiview",
+ "SPV_KHR_post_depth_coverage",
+ "SPV_KHR_non_semantic_info",
+ "SPV_KHR_shader_draw_parameters",
+ "SPV_KHR_ray_tracing",
+ "SPV_KHR_shader_clock",
+ "SPV_EXT_demote_to_helper_invocation",
+ "SPV_EXT_descriptor_indexing",
+ "SPV_EXT_fragment_fully_covered",
+ "SPV_EXT_fragment_invocation_density",
+ "SPV_EXT_fragment_shader_interlock",
+ "SPV_EXT_mesh_shader",
+ "SPV_EXT_shader_stencil_export",
+ "SPV_EXT_shader_viewport_index_layer",
+ // "SPV_AMD_shader_early_and_late_fragment_tests",
+ "SPV_GOOGLE_hlsl_functionality1",
+ "SPV_GOOGLE_user_type",
+ "SPV_KHR_ray_query",
+ "SPV_EXT_shader_image_int64",
+ "SPV_KHR_fragment_shader_barycentric",
+ "SPV_KHR_physical_storage_buffer",
+ "SPV_KHR_vulkan_memory_model",
+ // "SPV_KHR_compute_shader_derivatives",
+ // "SPV_KHR_maximal_reconvergence",
+ "SPV_KHR_float_controls",
+ "SPV_NV_shader_subgroup_partitioned",
+ // "SPV_KHR_quad_control"
+ };
+
+ if (SpvExtensionArg.starts_with("SPV_")) {
+ return ("+" + SpvExtensionArg).str();
+ }
+ if (SpvExtensionArg.compare_insensitive("DXC") == 0) {
+ bool first = true;
+ std::string Operand;
+ for (llvm::StringRef E : DxcSupportedExtensions) {
+ if (first) {
+ Operand = (Operand + "+" + E).str();
+ first = false;
+ continue;
+ }
+ Operand = (Operand + ",+" + E).str();
+ }
+ return Operand;
+ }
+ return SpvExtensionArg.str();
+}
+
std::string getSpirvExtArg(ArrayRef<std::string> SpvExtensionArgs) {
if (SpvExtensionArgs.empty()) {
return "-spirv-ext=all";
}
std::string LlvmOption =
- (Twine("-spirv-ext=+") + SpvExtensionArgs.front()).str();
+ "-spirv-ext=" + getSpirvExtOperand(SpvExtensionArgs[0]);
SpvExtensionArgs = SpvExtensionArgs.slice(1);
- for (auto Extension : SpvExtensionArgs) {
- if (Extension != "KHR")
- Extension = (Twine("+") + Extension).str();
- LlvmOption = (Twine(LlvmOption) + "," + Extension).str();
+ for (llvm::StringRef Extension : SpvExtensionArgs) {
+ LlvmOption =
+ (Twine(LlvmOption) + "," + getSpirvExtOperand(Extension)).str();
}
return LlvmOption;
}
bool isValidSPIRVExtensionName(const std::string &str) {
- std::regex pattern("KHR|SPV_[a-zA-Z0-9_]+");
+ std::regex pattern("dxc|DXC|KHR|SPV_[a-zA-Z0-9_]+");
return std::regex_match(str, pattern);
}
diff --git a/clang/test/Driver/dxc_fspv_extension.hlsl b/clang/test/Driver/dxc_fspv_extension.hlsl
index f09f21e8a72c9..0c9d3f5409984 100644
--- a/clang/test/Driver/dxc_fspv_extension.hlsl
+++ b/clang/test/Driver/dxc_fspv_extension.hlsl
@@ -14,6 +14,14 @@
// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=SPV_TEST1 -fspv-extension=KHR -fspv-extension=SPV_TEST2 2>&1 | FileCheck %s -check-prefix=TEST3
// TEST3: "-spirv-ext=+SPV_TEST1,KHR,+SPV_TEST2"
+// Merge KHR with other extensions.
+// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=KHR -fspv-extension=SPV_TEST2 2>&1 | FileCheck %s -check-prefix=TEST4
+// TEST4: "-spirv-ext=KHR,+SPV_TEST2"
+
+// Merge KHR with other extensions.
+// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=DXC 2>&1 | FileCheck %s -check-prefix=TEST5
+// TEST5: "-spirv-ext={{(\+SPV_[a-zA-Z0-9_]+,?)+}}"
+
// Check for the error message if the extension name is not properly formed.
// RUN: not %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=KHR_BAD -fspv-extension=TEST1 -fspv-extension=SPV_GOOD -fspv-extension=TEST2 2>&1 | FileCheck %s -check-prefix=FAIL
// FAIL: invalid value 'KHR_BAD' in '-fspv-extension'
diff --git a/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp b/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp
index 2726203d253ad..08fe8111a15b7 100644
--- a/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp
@@ -32,6 +32,24 @@ static const std::map<std::string, SPIRV::Extension::Extension, std::less<>>
SPIRV::Extension::Extension::SPV_EXT_arithmetic_fence},
{"SPV_EXT_demote_to_helper_invocation",
SPIRV::Extension::Extension::SPV_EXT_demote_to_helper_invocation},
+ {"SPV_EXT_descriptor_indexing",
+ SPIRV::Extension::Extension::SPV_EXT_descriptor_indexing},
+ {"SPV_EXT_fragment_fully_covered",
+ SPIRV::Extension::Extension::SPV_EXT_fragment_fully_covered},
+ {"SPV_EXT_fragment_invocation_density",
+ SPIRV::Extension::Extension::SPV_EXT_fragment_invocation_density},
+ {"SPV_EXT_fragment_shader_interlock",
+ SPIRV::Extension::Extension::SPV_EXT_fragment_shader_interlock},
+ {"SPV_EXT_mesh_shader",
+ SPIRV::Extension::Extension::SPV_EXT_mesh_shader},
+ {"SPV_EXT_shader_stencil_export",
+ SPIRV::Extension::Extension::SPV_EXT_shader_stencil_export},
+ {"SPV_EXT_shader_viewport_index_layer",
+ SPIRV::Extension::Extension::SPV_EXT_shader_viewport_index_layer},
+ {"SPV_GOOGLE_hlsl_functionality1",
+ SPIRV::Extension::Extension::SPV_GOOGLE_hlsl_functionality1},
+ {"SPV_GOOGLE_user_type",
+ SPIRV::Extension::Extension::SPV_GOOGLE_user_type},
{"SPV_INTEL_arbitrary_precision_integers",
SPIRV::Extension::Extension::SPV_INTEL_arbitrary_precision_integers},
{"SPV_INTEL_cache_controls",
@@ -57,6 +75,19 @@ static const std::map<std::string, SPIRV::Extension::Extension, std::less<>>
SPIRV::Extension::Extension::SPV_INTEL_memory_access_aliasing},
{"SPV_INTEL_joint_matrix",
SPIRV::Extension::Extension::SPV_INTEL_joint_matrix},
+ {"SPV_KHR_16bit_storage",
+ SPIRV::Extension::Extension::SPV_KHR_16bit_storage},
+ {"SPV_KHR_device_group",
+ SPIRV::Extension::Extension::SPV_KHR_device_group},
+ {"SPV_KHR_fragment_shading_rate",
+ SPIRV::Extension::Extension::SPV_KHR_fragment_shading_rate},
+ {"SPV_KHR_multiview", SPIRV::Extension::Extension::SPV_KHR_multiview},
+ {"SPV_KHR_post_depth_coverage",
+ SPIRV::Extension::Extension::SPV_KHR_post_depth_coverage},
+ {"SPV_KHR_shader_draw_parameters",
+ SPIRV::Extension::Extension::SPV_KHR_shader_draw_parameters},
+ {"SPV_KHR_ray_tracing",
+ SPIRV::Extension::Extension::SPV_KHR_ray_tracing},
{"SPV_KHR_uniform_group_instructions",
SPIRV::Extension::Extension::SPV_KHR_uniform_group_instructions},
{"SPV_KHR_no_integer_wrap_decoration",
@@ -89,6 +120,17 @@ static const std::map<std::string, SPIRV::Extension::Extension, std::less<>>
SPIRV::Extension::Extension::SPV_KHR_cooperative_matrix},
{"SPV_KHR_non_semantic_info",
SPIRV::Extension::Extension::SPV_KHR_non_semantic_info},
+ {"SPV_KHR_ray_query", SPIRV::Extension::Extension::SPV_KHR_ray_query},
+ {"SPV_EXT_shader_image_int64",
+ SPIRV::Extension::Extension::SPV_EXT_shader_image_int64},
+ {"SPV_KHR_fragment_shader_barycentric",
+ SPIRV::Extension::Extension::SPV_KHR_fragment_shader_barycentric},
+ {"SPV_KHR_physical_storage_buffer",
+ SPIRV::Extension::Extension::SPV_KHR_physical_storage_buffer},
+ {"SPV_KHR_vulkan_memory_model",
+ SPIRV::Extension::Extension::SPV_KHR_vulkan_memory_model},
+ {"SPV_NV_shader_subgroup_partitioned",
+ SPIRV::Extension::Extension::SPV_NV_shader_subgroup_partitioned},
{"SPV_INTEL_long_composites",
SPIRV::Extension::Extension::SPV_INTEL_long_composites},
{"SPV_INTEL_fp_max_error",
|
@llvm/pr-subscribers-backend-spir-v Author: Steven Perron (s-perron) ChangesThe default behaviour in DXC is to allow all extesions the compiler To avoid using a new extension when moving from DXC to Clang, we add the Full diff: https://github.com/llvm/llvm-project/pull/151554.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index eb53821bbf4d5..9471e9edcfd76 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -9428,8 +9428,12 @@ def fspv_target_env_EQ : Joined<["-"], "fspv-target-env=">, Group<dxc_Group>,
def fspv_extension_EQ
: Joined<["-"], "fspv-extension=">,
Group<dxc_Group>,
- HelpText<"Specify the available SPIR-V extensions. If this option is not "
- "specified, then all extensions are available.">;
+ HelpText<
+ "Specify the available SPIR-V extensions. If this option is not "
+ "specified, then all extensions are available. If KHR is specified, "
+ "then all KHR extensions will be available. If DXC is specified, "
+ "then all extensions implemented by the DirectXShader compiler will "
+ "be available. This option is useful for moving from DXC to Clang.">;
def fvk_use_dx_layout
: DXCFlag<"fvk-use-dx-layout">,
HelpText<"Use DirectX memory layout for Vulkan resources.">;
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 38f4643abad98..48761c254dfd8 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -173,24 +173,78 @@ bool isLegalValidatorVersion(StringRef ValVersionStr, const Driver &D) {
return true;
}
+std::string getSpirvExtOperand(llvm::StringRef SpvExtensionArg) {
+ // The extensions that are commented out are supported in DXC, but the SPIR-V
+ // backend does not know about them yet.
+ static const std::vector<std::string> DxcSupportedExtensions = {
+ "SPV_KHR_16bit_storage",
+ "SPV_KHR_device_group",
+ "SPV_KHR_fragment_shading_rate",
+ "SPV_KHR_multiview",
+ "SPV_KHR_post_depth_coverage",
+ "SPV_KHR_non_semantic_info",
+ "SPV_KHR_shader_draw_parameters",
+ "SPV_KHR_ray_tracing",
+ "SPV_KHR_shader_clock",
+ "SPV_EXT_demote_to_helper_invocation",
+ "SPV_EXT_descriptor_indexing",
+ "SPV_EXT_fragment_fully_covered",
+ "SPV_EXT_fragment_invocation_density",
+ "SPV_EXT_fragment_shader_interlock",
+ "SPV_EXT_mesh_shader",
+ "SPV_EXT_shader_stencil_export",
+ "SPV_EXT_shader_viewport_index_layer",
+ // "SPV_AMD_shader_early_and_late_fragment_tests",
+ "SPV_GOOGLE_hlsl_functionality1",
+ "SPV_GOOGLE_user_type",
+ "SPV_KHR_ray_query",
+ "SPV_EXT_shader_image_int64",
+ "SPV_KHR_fragment_shader_barycentric",
+ "SPV_KHR_physical_storage_buffer",
+ "SPV_KHR_vulkan_memory_model",
+ // "SPV_KHR_compute_shader_derivatives",
+ // "SPV_KHR_maximal_reconvergence",
+ "SPV_KHR_float_controls",
+ "SPV_NV_shader_subgroup_partitioned",
+ // "SPV_KHR_quad_control"
+ };
+
+ if (SpvExtensionArg.starts_with("SPV_")) {
+ return ("+" + SpvExtensionArg).str();
+ }
+ if (SpvExtensionArg.compare_insensitive("DXC") == 0) {
+ bool first = true;
+ std::string Operand;
+ for (llvm::StringRef E : DxcSupportedExtensions) {
+ if (first) {
+ Operand = (Operand + "+" + E).str();
+ first = false;
+ continue;
+ }
+ Operand = (Operand + ",+" + E).str();
+ }
+ return Operand;
+ }
+ return SpvExtensionArg.str();
+}
+
std::string getSpirvExtArg(ArrayRef<std::string> SpvExtensionArgs) {
if (SpvExtensionArgs.empty()) {
return "-spirv-ext=all";
}
std::string LlvmOption =
- (Twine("-spirv-ext=+") + SpvExtensionArgs.front()).str();
+ "-spirv-ext=" + getSpirvExtOperand(SpvExtensionArgs[0]);
SpvExtensionArgs = SpvExtensionArgs.slice(1);
- for (auto Extension : SpvExtensionArgs) {
- if (Extension != "KHR")
- Extension = (Twine("+") + Extension).str();
- LlvmOption = (Twine(LlvmOption) + "," + Extension).str();
+ for (llvm::StringRef Extension : SpvExtensionArgs) {
+ LlvmOption =
+ (Twine(LlvmOption) + "," + getSpirvExtOperand(Extension)).str();
}
return LlvmOption;
}
bool isValidSPIRVExtensionName(const std::string &str) {
- std::regex pattern("KHR|SPV_[a-zA-Z0-9_]+");
+ std::regex pattern("dxc|DXC|KHR|SPV_[a-zA-Z0-9_]+");
return std::regex_match(str, pattern);
}
diff --git a/clang/test/Driver/dxc_fspv_extension.hlsl b/clang/test/Driver/dxc_fspv_extension.hlsl
index f09f21e8a72c9..0c9d3f5409984 100644
--- a/clang/test/Driver/dxc_fspv_extension.hlsl
+++ b/clang/test/Driver/dxc_fspv_extension.hlsl
@@ -14,6 +14,14 @@
// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=SPV_TEST1 -fspv-extension=KHR -fspv-extension=SPV_TEST2 2>&1 | FileCheck %s -check-prefix=TEST3
// TEST3: "-spirv-ext=+SPV_TEST1,KHR,+SPV_TEST2"
+// Merge KHR with other extensions.
+// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=KHR -fspv-extension=SPV_TEST2 2>&1 | FileCheck %s -check-prefix=TEST4
+// TEST4: "-spirv-ext=KHR,+SPV_TEST2"
+
+// Merge KHR with other extensions.
+// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=DXC 2>&1 | FileCheck %s -check-prefix=TEST5
+// TEST5: "-spirv-ext={{(\+SPV_[a-zA-Z0-9_]+,?)+}}"
+
// Check for the error message if the extension name is not properly formed.
// RUN: not %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=KHR_BAD -fspv-extension=TEST1 -fspv-extension=SPV_GOOD -fspv-extension=TEST2 2>&1 | FileCheck %s -check-prefix=FAIL
// FAIL: invalid value 'KHR_BAD' in '-fspv-extension'
diff --git a/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp b/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp
index 2726203d253ad..08fe8111a15b7 100644
--- a/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp
@@ -32,6 +32,24 @@ static const std::map<std::string, SPIRV::Extension::Extension, std::less<>>
SPIRV::Extension::Extension::SPV_EXT_arithmetic_fence},
{"SPV_EXT_demote_to_helper_invocation",
SPIRV::Extension::Extension::SPV_EXT_demote_to_helper_invocation},
+ {"SPV_EXT_descriptor_indexing",
+ SPIRV::Extension::Extension::SPV_EXT_descriptor_indexing},
+ {"SPV_EXT_fragment_fully_covered",
+ SPIRV::Extension::Extension::SPV_EXT_fragment_fully_covered},
+ {"SPV_EXT_fragment_invocation_density",
+ SPIRV::Extension::Extension::SPV_EXT_fragment_invocation_density},
+ {"SPV_EXT_fragment_shader_interlock",
+ SPIRV::Extension::Extension::SPV_EXT_fragment_shader_interlock},
+ {"SPV_EXT_mesh_shader",
+ SPIRV::Extension::Extension::SPV_EXT_mesh_shader},
+ {"SPV_EXT_shader_stencil_export",
+ SPIRV::Extension::Extension::SPV_EXT_shader_stencil_export},
+ {"SPV_EXT_shader_viewport_index_layer",
+ SPIRV::Extension::Extension::SPV_EXT_shader_viewport_index_layer},
+ {"SPV_GOOGLE_hlsl_functionality1",
+ SPIRV::Extension::Extension::SPV_GOOGLE_hlsl_functionality1},
+ {"SPV_GOOGLE_user_type",
+ SPIRV::Extension::Extension::SPV_GOOGLE_user_type},
{"SPV_INTEL_arbitrary_precision_integers",
SPIRV::Extension::Extension::SPV_INTEL_arbitrary_precision_integers},
{"SPV_INTEL_cache_controls",
@@ -57,6 +75,19 @@ static const std::map<std::string, SPIRV::Extension::Extension, std::less<>>
SPIRV::Extension::Extension::SPV_INTEL_memory_access_aliasing},
{"SPV_INTEL_joint_matrix",
SPIRV::Extension::Extension::SPV_INTEL_joint_matrix},
+ {"SPV_KHR_16bit_storage",
+ SPIRV::Extension::Extension::SPV_KHR_16bit_storage},
+ {"SPV_KHR_device_group",
+ SPIRV::Extension::Extension::SPV_KHR_device_group},
+ {"SPV_KHR_fragment_shading_rate",
+ SPIRV::Extension::Extension::SPV_KHR_fragment_shading_rate},
+ {"SPV_KHR_multiview", SPIRV::Extension::Extension::SPV_KHR_multiview},
+ {"SPV_KHR_post_depth_coverage",
+ SPIRV::Extension::Extension::SPV_KHR_post_depth_coverage},
+ {"SPV_KHR_shader_draw_parameters",
+ SPIRV::Extension::Extension::SPV_KHR_shader_draw_parameters},
+ {"SPV_KHR_ray_tracing",
+ SPIRV::Extension::Extension::SPV_KHR_ray_tracing},
{"SPV_KHR_uniform_group_instructions",
SPIRV::Extension::Extension::SPV_KHR_uniform_group_instructions},
{"SPV_KHR_no_integer_wrap_decoration",
@@ -89,6 +120,17 @@ static const std::map<std::string, SPIRV::Extension::Extension, std::less<>>
SPIRV::Extension::Extension::SPV_KHR_cooperative_matrix},
{"SPV_KHR_non_semantic_info",
SPIRV::Extension::Extension::SPV_KHR_non_semantic_info},
+ {"SPV_KHR_ray_query", SPIRV::Extension::Extension::SPV_KHR_ray_query},
+ {"SPV_EXT_shader_image_int64",
+ SPIRV::Extension::Extension::SPV_EXT_shader_image_int64},
+ {"SPV_KHR_fragment_shader_barycentric",
+ SPIRV::Extension::Extension::SPV_KHR_fragment_shader_barycentric},
+ {"SPV_KHR_physical_storage_buffer",
+ SPIRV::Extension::Extension::SPV_KHR_physical_storage_buffer},
+ {"SPV_KHR_vulkan_memory_model",
+ SPIRV::Extension::Extension::SPV_KHR_vulkan_memory_model},
+ {"SPV_NV_shader_subgroup_partitioned",
+ SPIRV::Extension::Extension::SPV_NV_shader_subgroup_partitioned},
{"SPV_INTEL_long_composites",
SPIRV::Extension::Extension::SPV_INTEL_long_composites},
{"SPV_INTEL_fp_max_error",
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp -- clang/lib/Driver/ToolChains/HLSL.cpp llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp View the diff from clang-format here.diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index bc51d7f37..d74e9c9e6 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -177,35 +177,23 @@ void getSpirvExtOperand(StringRef SpvExtensionArg, raw_ostream &out) {
// The extensions that are commented out are supported in DXC, but the SPIR-V
// backend does not know about them yet.
static const std::vector<StringRef> DxcSupportedExtensions = {
- "SPV_KHR_16bit_storage",
- "SPV_KHR_device_group",
- "SPV_KHR_fragment_shading_rate",
- "SPV_KHR_multiview",
- "SPV_KHR_post_depth_coverage",
- "SPV_KHR_non_semantic_info",
- "SPV_KHR_shader_draw_parameters",
- "SPV_KHR_ray_tracing",
- "SPV_KHR_shader_clock",
- "SPV_EXT_demote_to_helper_invocation",
- "SPV_EXT_descriptor_indexing",
- "SPV_EXT_fragment_fully_covered",
+ "SPV_KHR_16bit_storage", "SPV_KHR_device_group",
+ "SPV_KHR_fragment_shading_rate", "SPV_KHR_multiview",
+ "SPV_KHR_post_depth_coverage", "SPV_KHR_non_semantic_info",
+ "SPV_KHR_shader_draw_parameters", "SPV_KHR_ray_tracing",
+ "SPV_KHR_shader_clock", "SPV_EXT_demote_to_helper_invocation",
+ "SPV_EXT_descriptor_indexing", "SPV_EXT_fragment_fully_covered",
"SPV_EXT_fragment_invocation_density",
- "SPV_EXT_fragment_shader_interlock",
- "SPV_EXT_mesh_shader",
- "SPV_EXT_shader_stencil_export",
- "SPV_EXT_shader_viewport_index_layer",
+ "SPV_EXT_fragment_shader_interlock", "SPV_EXT_mesh_shader",
+ "SPV_EXT_shader_stencil_export", "SPV_EXT_shader_viewport_index_layer",
// "SPV_AMD_shader_early_and_late_fragment_tests",
- "SPV_GOOGLE_hlsl_functionality1",
- "SPV_GOOGLE_user_type",
- "SPV_KHR_ray_query",
- "SPV_EXT_shader_image_int64",
- "SPV_KHR_fragment_shader_barycentric",
- "SPV_KHR_physical_storage_buffer",
+ "SPV_GOOGLE_hlsl_functionality1", "SPV_GOOGLE_user_type",
+ "SPV_KHR_ray_query", "SPV_EXT_shader_image_int64",
+ "SPV_KHR_fragment_shader_barycentric", "SPV_KHR_physical_storage_buffer",
"SPV_KHR_vulkan_memory_model",
// "SPV_KHR_compute_shader_derivatives",
// "SPV_KHR_maximal_reconvergence",
- "SPV_KHR_float_controls",
- "SPV_NV_shader_subgroup_partitioned",
+ "SPV_KHR_float_controls", "SPV_NV_shader_subgroup_partitioned",
// "SPV_KHR_quad_control"
};
@@ -431,7 +419,7 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
std::vector<std::string> SpvExtensionArgs =
Args.getAllArgValues(options::OPT_fspv_extension_EQ);
if (checkExtensionArgsAreValid(SpvExtensionArgs, getDriver())) {
- SmallString<1024> LlvmOption = getSpirvExtArg(SpvExtensionArgs);
+ SmallString<1024> LlvmOption = getSpirvExtArg(SpvExtensionArgs);
DAL->AddSeparateArg(nullptr, Opts.getOption(options::OPT_mllvm),
LlvmOption);
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple nits to take or leave, otherwise this looks reasonable to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the formatting issue on the CI and 2 nits
} | ||
return LlvmOption; | ||
} | ||
|
||
bool isValidSPIRVExtensionName(const std::string &str) { | ||
std::regex pattern("KHR|SPV_[a-zA-Z0-9_]+"); | ||
std::regex pattern("dxc|DXC|KHR|SPV_[a-zA-Z0-9_]+"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::regex pattern("dxc|DXC|KHR|SPV_[a-zA-Z0-9_]+"); | |
std::regex pattern("dxc|DXC|khr|KHR|SPV_[a-zA-Z0-9_]+"); |
// RUN: %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=KHR -fspv-extension=SPV_TEST2 2>&1 | FileCheck %s -check-prefix=TEST4 | ||
// TEST4: "-spirv-ext=KHR,+SPV_TEST2" | ||
|
||
// Merge KHR with other extensions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: comment seems wrong
// Merge KHR with other extensions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still looks good to me. A few style nits since we don't need to explicitly specify the llvm namespace in this file.
Co-authored-by: Chris B <[email protected]>
Co-authored-by: Chris B <[email protected]>
Co-authored-by: Chris B <[email protected]>
Co-authored-by: Chris B <[email protected]>
Co-authored-by: Chris B <[email protected]>
Co-authored-by: Chris B <[email protected]>
The default behaviour in DXC is to allow all extesions the compiler
knows about. We did the same in clang: all extensions that clang knows
about. However, this causes the shader to use different extensions
because the two compilers have different sets of extensions.
To avoid using a new extension when moving from DXC to Clang, we add the
special DXC suboptions to
-fspv-extension
. If-fspv-extension=DXC
isused, then the available extensions will be those available in DXC.